Skip to content

ui: add invalid & single select support in autocomplete#26803

Merged
harsh-vador merged 5 commits intomainfrom
autocomplete-invalid-state
Mar 27, 2026
Merged

ui: add invalid & single select support in autocomplete#26803
harsh-vador merged 5 commits intomainfrom
autocomplete-invalid-state

Conversation

@harsh-vador
Copy link
Copy Markdown
Contributor

@harsh-vador harsh-vador commented Mar 26, 2026

Describe your changes:

I worked on adding support for single and multiple autocomplete support along with handling of error state which highlights the border 'red' when received isInvalid

Screen.Recording.2026-03-27.at.12.25.08.AM.mov

Type of change:

  • Bug fix
  • Improvement
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 26, 2026

🟡 Playwright Results — all passed (14 flaky)

✅ 3402 passed · ❌ 0 failed · 🟡 14 flaky · ⏭️ 216 skipped

Shard Passed Failed Flaky Skipped
🟡 Shard 1 454 0 1 2
🟡 Shard 2 600 0 4 32
🟡 Shard 3 605 0 4 27
🟡 Shard 4 601 0 2 47
✅ Shard 5 587 0 0 67
🟡 Shard 6 555 0 3 41
🟡 14 flaky test(s) (passed on retry)
  • Pages/UserCreationWithPersona.spec.ts › Create user with persona and verify on profile (shard 1, 1 retry)
  • Features/BulkEditEntity.spec.ts › Glossary (shard 2, 1 retry)
  • Features/DataQuality/TestCaseImportExportE2eFlow.spec.ts › Admin: Complete export-import-validate flow (shard 2, 1 retry)
  • Features/DataQuality/TestCaseIncidentPermissions.spec.ts › User with only VIEW cannot PATCH incidents (shard 2, 1 retry)
  • Features/DataQuality/TestCaseResultPermissions.spec.ts › User with TEST_CASE.EDIT_ALL can see edit action on test case (shard 2, 1 retry)
  • Features/Permissions/GlossaryPermissions.spec.ts › Team-based permissions work correctly (shard 3, 1 retry)
  • Features/RestoreEntityInheritedFields.spec.ts › Validate restore with Inherited domain and data products assigned (shard 3, 1 retry)
  • Features/UserProfileOnlineStatus.spec.ts › Should not show online status for inactive users (shard 3, 1 retry)
  • Flow/ExploreDiscovery.spec.ts › Should display deleted assets when showDeleted is checked and deleted is not present in queryFilter (shard 3, 1 retry)
  • Pages/Customproperties-part2.spec.ts › entityReferenceList shows item count, scrollable list, no expand toggle (shard 4, 1 retry)
  • Pages/Entity.spec.ts › Tag Add, Update and Remove (shard 4, 1 retry)
  • Pages/Users.spec.ts › Permissions for table details page for Data Consumer (shard 6, 1 retry)
  • Pages/Users.spec.ts › Check permissions for Data Steward (shard 6, 1 retry)
  • VersionPages/EntityVersionPages.spec.ts › Directory (shard 6, 1 retry)

📦 Download artifacts

How to debug locally
# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

@harsh-vador
Copy link
Copy Markdown
Contributor Author

@gitar-bot i have replied to your comments, please update accordingly

)}>
<AriaInput
className="tw:w-full tw:flex-[1_0_0] tw:appearance-none tw:bg-transparent tw:text-md tw:text-ellipsis tw:text-primary tw:caret-alpha-black/90 tw:outline-none tw:placeholder:text-placeholder tw:focus:outline-hidden tw:disabled:cursor-not-allowed tw:disabled:text-disabled tw:disabled:placeholder:text-disabled"
className="tw:w-full tw:flex-[1_0_0] tw:appearance-none tw:bg-transparent tw:text-sm tw:text-ellipsis tw:text-primary tw:caret-alpha-black/90 tw:outline-none tw:placeholder:text-placeholder tw:focus:outline-hidden tw:disabled:cursor-not-allowed tw:disabled:text-disabled tw:disabled:placeholder:text-disabled"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood, thanks for the clarification. The tw:text-sm change is intentional.

@gitar-bot
Copy link
Copy Markdown

gitar-bot bot commented Mar 27, 2026

@gitar-bot i have replied to your comments, please update accordingly

Thanks for the clarifications! Both findings have been acknowledged:

  • The menuTrigger: "input" change is intentional.
  • The tw:text-sm font size change is intentional.

No further action needed from my side.

@harsh-vador
Copy link
Copy Markdown
Contributor Author

@gitar-bot share the final status of code review

@gitar-bot
Copy link
Copy Markdown

gitar-bot bot commented Mar 27, 2026

@gitar-bot share the final status of code review

Here's the final code review status:

2 findings were identified, both marked as intentional:

  1. menuTrigger change from "focus" to "input" — Initially flagged as a potential UX regression, but confirmed intentional by you.
  2. Input text size change from tw:text-md to tw:text-sm — Initially flagged as an unintentional side-effect, but confirmed intentional by you.

No outstanding issues remain. The PR is clear from a review standpoint.

@harsh-vador harsh-vador changed the title ui: invalid state support ui: add invalid & single select support in autocomplete Mar 27, 2026
@harsh-vador harsh-vador enabled auto-merge (squash) March 27, 2026 09:15
@gitar-bot
Copy link
Copy Markdown

gitar-bot bot commented Mar 27, 2026

Code Review 👍 Approved with suggestions 1 resolved / 2 findings

Adds invalid state and single-select support to the autocomplete component with comprehensive error handling. Consider reverting the input text size reduction from text-md to text-sm, which affects all autocomplete consumers beyond the new functionality.

💡 Bug: Input text size changed from text-md to text-sm for all instances

📄 openmetadata-ui-core-components/src/main/resources/ui/src/components/base/autocomplete/autocomplete.tsx:257

Line 257 changes the input class from tw:text-md to tw:text-sm, which reduces the font size of the autocomplete input for all consumers — not just the new single-select or invalid-state functionality. This appears to be an unintentional side-effect unrelated to the PR's purpose (invalid state support). If intentional, it should be called out in the PR description as a visual change.

✅ 1 resolved
Bug: menuTrigger change from "focus" to "input" is a behavioral regression

📄 openmetadata-ui-core-components/src/main/resources/ui/src/components/base/autocomplete/autocomplete.tsx:452
Changing menuTrigger from "focus" to "input" (line 452) means the dropdown no longer opens when the user clicks/tabs into the autocomplete — it now only opens when the user starts typing. This is a UX regression for all existing Autocomplete consumers (12+ story usages, plus production code), not just the new single-select mode.

The added ArrowDown handler partially compensates by letting users manually open the dropdown, but this is far less discoverable than the previous focus-to-open behavior.

If the intent was to prevent the dropdown from reopening in single-select mode after a selection is made, consider conditioning this on the multiple prop instead of changing it globally.

🤖 Prompt for agents
Code Review: Adds invalid state and single-select support to the autocomplete component with comprehensive error handling. Consider reverting the input text size reduction from text-md to text-sm, which affects all autocomplete consumers beyond the new functionality.

1. 💡 Bug: Input text size changed from text-md to text-sm for all instances
   Files: openmetadata-ui-core-components/src/main/resources/ui/src/components/base/autocomplete/autocomplete.tsx:257

   Line 257 changes the input class from `tw:text-md` to `tw:text-sm`, which reduces the font size of the autocomplete input for all consumers — not just the new single-select or invalid-state functionality. This appears to be an unintentional side-effect unrelated to the PR's purpose (invalid state support). If intentional, it should be called out in the PR description as a visual change.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@harsh-vador harsh-vador merged commit d5ceaea into main Mar 27, 2026
42 checks passed
@harsh-vador harsh-vador deleted the autocomplete-invalid-state branch March 27, 2026 11:02
harsh-vador added a commit that referenced this pull request Mar 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test Add this label to run secure Github workflows on PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants